Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the BrushFilter interaction in src/interaction/brushFilter.ts to prevent overriding the y-scale domain for marks with independent y-scales, thereby avoiding cross-scale domain pollution. A review comment suggests making the detection of independent scales more robust by checking for scale keys in addition to the independent flag, potentially utilizing existing utility functions for consistency.
| const newMarks = marks.map((mark) => { | ||
| // Independent y scale manages its own domain based on filtered data, | ||
| // so skip overriding it to avoid cross-scale domain pollution. | ||
| const isYIndependent = mark.scale?.y?.independent === true; |
There was a problem hiding this comment.
The current logic to detect an independent Y scale only checks for independent: true. This is correct, but might be incomplete. The codebase has other ways to define independent scales, for example by using different key properties on the scale.
To make this fix more robust and align with other parts of the codebase (like hasIndependentXYScale in utils.ts), consider also checking for scale keys. This would handle cases where dual axes are defined by keys without explicitly setting independent: true.
For example, you could modify the logic like this:
// At the top of the file:
import { hasIndependentXYScale } from './utils';
// ...
// Inside setState, before mapping marks:
const hasMultiYScales = hasIndependentXYScale('y', marks);
// ...
// Inside map, replace the isYIndependent check:
const isYIndependent =
mark.scale?.y?.independent === true ||
(hasMultiYScales && !!mark.scale?.y?.key);1 participant
Checklist
npm testpassesDescription of change
#7272